MDEV-40480 LOAD DATA leaves a stale STORED generated column after a B… - #5470
MDEV-40480 LOAD DATA leaves a stale STORED generated column after a B…#5470midenok wants to merge 1 commit into
Conversation
…EFORE INSERT trigger changes its base column On the LOAD DATA path the base columns are filled directly from the input file and fill_record_n_invoke_before_triggers() is then called with an empty field list (there is no SET clause). After the BEFORE INSERT trigger changed a base column, the stored generated columns were not recomputed, so they kept the value derived from the pre-trigger input (e.g. g=24 instead of 40 for g=v*2 with v set to 20 by the trigger). A regular INSERT was unaffected. The recompute was guarded by "fields.elements". That condition is a leftover from the original computed-columns implementation (f7a75b9), where fill_record_n_invoke_before_triggers() had no TABLE* argument and had to reverse-derive the table from the first item of the field list: if (fields.elements) { fld= (Item_field*)f++; item_field= fld->field_for_view_update(); table= item_field->field->table; ... } With an empty field list there was no way to obtain the table, so the recompute was silently skipped. Since bc4a456 (MDEV-452) the function receives TABLE* explicitly, which made the whole derivation dead code (as the in-place DBUG_ASSERT(table == item_field->field->table) confirmed). Recompute the virtual fields unconditionally on table->vfield, the same way the Field** overload of fill_record_n_invoke_before_triggers() already does.
|
|
There was a problem hiding this comment.
Pull request overview
Fixes MDEV-40480 where LOAD DATA could leave STORED generated columns stale if a BEFORE INSERT trigger modifies a base column. The change aligns the List<Item> overload of fill_record_n_invoke_before_triggers() with the existing Field** overload by recomputing virtual/generated fields whenever the table has virtual fields (table->vfield), independent of whether the SET-clause field list is empty.
Changes:
- Remove dead/legacy logic that derived
TABLE*fromfields.head()and skipped recomputation whenfields.elements == 0. - Always recompute virtual/generated fields after BEFORE triggers when
table->vfieldis present. - Add a regression test covering
LOAD DATA+BEFORE INSERTtrigger + STORED generated column recomputation.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| sql/sql_base.cc | Recomputes virtual/generated fields after BEFORE triggers whenever table->vfield is set, fixing the LOAD DATA empty-field-list case. |
| mysql-test/suite/gcol/t/gcol_bugfixes.test | Adds a LOAD DATA regression test reproducing the stale STORED generated column scenario. |
| mysql-test/suite/gcol/r/gcol_bugfixes.result | Updates expected output for the new regression test. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
…EFORE INSERT trigger changes its base column
On the LOAD DATA path the base columns are filled directly from the input file and fill_record_n_invoke_before_triggers() is then called with an empty field list (there is no SET clause). After the BEFORE INSERT trigger changed a base column, the stored generated columns were not recomputed, so they kept the value derived from the pre-trigger input (e.g. g=24 instead of 40 for g=v*2 with v set to 20 by the trigger). A regular INSERT was unaffected.
The recompute was guarded by "fields.elements". That condition is a leftover from the original computed-columns implementation (f7a75b9), where fill_record_n_invoke_before_triggers() had no TABLE* argument and had to reverse-derive the table from the first item of the field list:
With an empty field list there was no way to obtain the table, so the recompute was silently skipped. Since bc4a456 (MDEV-452) the function receives TABLE* explicitly, which made the whole derivation dead code (as the in-place DBUG_ASSERT(table == item_field->field->table) confirmed). Recompute the virtual fields unconditionally on table->vfield, the same way the Field** overload of fill_record_n_invoke_before_triggers() already does.